Rewrite of instance.md, introducing data-methods.md #514

skirtles-code · 2020-09-20T00:29:58Z

This could take a while...

Introduction

This is intended as a discussion starter, not necessarily something that should be merged. I could have just opened an issue but I don't think I could have properly conveyed what I wanted to say. Rather than just criticising the current version of instance.md I've tried to fix the problems myself. Opinions may vary as to whether I've succeeded.

To be clear, this definitely shouldn't be merged in its present form. At the very least I'll need to go through other pages to update any links. I won't bother doing that unless there's a realistic chance of it being merged. Update: I'm now happy for it to be merged.

I've gone through and added inline review comments to this PR. I think they're necessary but they do make reading the finished files a bit difficult. I suggest reading the files without my comments first to get an overall feel for the finished result.

The diff isn't necessarily super helpful. While my version does borrow some small passages from the current version it's mostly a rewrite. Some of the structure and narrative are similar to the previous version but the wording is mostly new. The exceptions are the two sections about the lifecycle, which are almost unchanged.

The relevant pages are available at:

The Problem

instance.md has evolved over many years. But what is it actually about? What are we trying to explain and why?

I feel like it's been tweaked and patched so many times that the original narrative has become somewhat blurred.

Vue 3 then introduced new problems. The concept of the 'Vue instance' doesn't really exist anymore. The closest match is a 'component instance', which behaves much the same as a vm in Vue 2.

On its own that wouldn't be a problem but the introduction of application instances makes it much, much more difficult to explain what's going on. Trying to write an explanation that navigates through all the potential misunderstandings without getting lost in the weeds is not at all easy.

In my opinion, the current version doesn't make a clear enough distinction between application instances and component instances. The term 'component instance' isn't even used but I think it's unavoidable to qualify the word 'instance' in some way. I don't think there's any reason to avoid the word 'component', the basic concept having been introduced in the guide's Introduction.

My Attempted Solution

I've had a go at writing my own explanation of these topics. I've also moved some material out to a new page, called data-methods.md.

Whether this is an improvement over the current documentation is subjective. I prefer it (as a totally impartial judge). Whether you agree or not, hopefully it'll help as a discussion starter to find the right balance.

Please feel free to focus any feedback on the bits you don't like. No sugar coating required.

NataliaTepluhina · 2020-09-21T03:51:18Z

@skirtles-code thank you for bringing this! From a quick glance, it makes a lot of sense to me, will look in detail today

NataliaTepluhina

First of all, thank you for the huge effort that you put into this rewrite, @skirtles-code! It's a very thoughtful addition and I believe we should merge it.

I've added a few notes and nitpicks for your consideration, would be happy to discuss!

src/.vuepress/config.js

src/guide/data-methods.md

src/guide/instance.md

skirtles-code

@NataliaTepluhina Sorry, I've made a right mess of this review process.

When I first created this PR I went through and carefully annotated my changes to explain exactly what I'd done and why. I didn't want anyone to have to try to review it without a proper explanation from me.

However, I now realise that I'm the only person who could see those comments. Oops.

I'm going to click the big green button and hopefully that'll make my original comments visible to everybody. I stress that these were all written prior to your feedback, which I will now go through and address individually.

Sorry about that.

src/.vuepress/config.js

src/guide/data-methods.md

src/guide/instance.md

src/guide/template-syntax.md

skirtles-code · 2020-09-23T15:35:17Z

@NataliaTepluhina As far as I'm concerned this is now good to be merged.

I haven't marked the conversations as Resolved as I suspect that'll make them harder to read but I can go through and mark all mine as resolved if you want.

Let me know if you'd prefer further changes.

NataliaTepluhina

Incredible work, thank you so much @skirtles-code for all the detailed explanations you added to your changes! I have a few additional minor tweaks but in general changes look good to me 👍🏻

src/guide/data-methods.md

NataliaTepluhina · 2020-09-24T03:44:21Z

src/guide/data-methods.md

+In cases where a component is only used once, the debouncing can be applied directly within `methods`:
+
+```js
+const app = Vue.createApp({


We need to show importing _ from lodash in this code snippet as it's not completely clear to people without lodash experience what is an underscore here

I've updated this example to try to address this. It isn't quite as straightforward as it sounded because (I think) it needs to use a CDN version of Lodash for consistency with the other examples. That leaves the _ not explicitly defined. I've added a comment in the code to try to clarify a bit more.

phanan · 2020-09-24T07:49:01Z

src/guide/data-methods.md

+
+## Methods
+
+You might be tempted to put functions in your `data` so that you can use them as methods:


I've never encountered anyone who does this (putting functions in data as methods), no matter how much of a beginner they are so IMHO this might not be the best approach to introduce methods.

Yeah, I made a similar observation here: https://github.com/vuejs/docs-next/pull/514/files/932839b6f612d79f299ab7e45604e0188d327e96#r491639058

I'll give it some more thought. I probably just need to delete most of that introduction and get to the point.

While this is theoretically possible, I'm think this would introduce an anti-pattern and may be more harmful than helpful. However, I do think this would make an interesting blog post!

I've rewritten the Methods section. A few small pieces remain but it's mostly new.

As a result, the sections on Data Properties and Methods aren't really linked anymore. They could be split into two pages, though they would be quite short compared to the rest of the guide.

I've given up on trying to explain that the properties created by methods aren't reactive. There are plenty of more important points to hit.

bencodezen · 2020-09-25T11:45:09Z

src/guide/data-methods.md

+
+## Data Properties
+
+The `data` function for a component is called as part of creating a new component instance. It should return an object, which Vue wraps in its reactivity system and stores on the instance as `$data`. For convenience, any top-level properties of that object are also exposed directly via the component instance:


Suggested change

The `data` function for a component is called as part of creating a new component instance. It should return an object, which Vue wraps in its reactivity system and stores on the instance as `$data`. For convenience, any top-level properties of that object are also exposed directly via the component instance:

The `data` property for a component is called as part of creating a new component instance. It is a function that should return an object, which Vue will then wrap in its reactivity system and store on the component instance as `$data`. For convenience, any top-level properties of that object are also exposed directly via the component instance:

Suggested edits are motivated by people not as familiar with JS, so calling it a property would be easier to follow initially, and then defining it technically should allow them that gradual ease into the concepts for data.

The later parts are now integrated.

I don't think we can open the paragraph with 'The data property' because that would conflict with the more common meaning of 'data property'.

I've had a go at rewording the start of this paragraph to take things slower.

bencodezen

Thanks for the detailed work on this @skirtles-code! Looking forward to giving this another look once the changes and discussion have been resolved!

skirtles-code · 2020-09-28T22:14:18Z

@NataliaTepluhina @phanan @bencodezen Thanks for all the feedback. I think I've addressed all the concerns raised so far.

The Methods section is almost completely rewritten since you last saw it.

NataliaTepluhina

@skirtles-code thanks for Methods rewrite! Looks good to me 👍🏻

NataliaTepluhina · 2020-10-04T11:29:33Z

@phanan @bencodezen I am going to merge this one and if you want more changes to the doc, let's make it iteratively on follow-up PRs

* docs: experimental rewrite of instance.md, introducing data-methods.md * docs: cut material from instance.md that isn't on topic * docs: move data-methods.md to after template-syntax.md * fix: change 'application' to 'component' in template-syntax.md * fix: use bold when introducing new terms in instance.md * docs: rewrite data-methods.md

docs: experimental rewrite of instance.md, introducing data-methods.md

952f8cb

skirtles-code marked this pull request as ready for review September 20, 2020 03:52

NataliaTepluhina added the discussion Topics to discuss that don't have clear action items yet label Sep 20, 2020

NataliaTepluhina reviewed Sep 22, 2020

View reviewed changes

src/.vuepress/config.js Outdated Show resolved Hide resolved

src/guide/data-methods.md Show resolved Hide resolved

src/guide/instance.md Outdated Show resolved Hide resolved

skirtles-code commented Sep 22, 2020

View reviewed changes

skirtles-code self-assigned this Sep 23, 2020

skirtles-code added 2 commits September 23, 2020 13:13

docs: cut material from instance.md that isn't on topic

ac1a710

docs: move data-methods.md to after template-syntax.md

932839b

skirtles-code changed the title ~~Experimental rewrite of instance.md, introducing data-methods.md~~ Rewrite of instance.md, introducing data-methods.md Sep 23, 2020

skirtles-code commented Sep 23, 2020

View reviewed changes

src/guide/template-syntax.md Show resolved Hide resolved

skirtles-code assigned NataliaTepluhina and unassigned skirtles-code Sep 23, 2020

NataliaTepluhina reviewed Sep 24, 2020

View reviewed changes

NataliaTepluhina requested review from phanan and bencodezen September 24, 2020 03:47

phanan reviewed Sep 24, 2020

View reviewed changes

skirtles-code assigned skirtles-code and unassigned NataliaTepluhina Sep 24, 2020

bencodezen reviewed Sep 25, 2020

View reviewed changes

bencodezen requested changes Sep 25, 2020

View reviewed changes

skirtles-code added 3 commits September 28, 2020 21:43

fix: change 'application' to 'component' in template-syntax.md

38c872c

fix: use bold when introducing new terms in instance.md

d7da73e

docs: rewrite data-methods.md

6424ce7

skirtles-code requested review from phanan, NataliaTepluhina and bencodezen September 28, 2020 22:14

skirtles-code removed their assignment Sep 29, 2020

NataliaTepluhina approved these changes Sep 30, 2020

View reviewed changes

NataliaTepluhina merged commit ab4fac3 into vuejs:master Oct 4, 2020

GuiDevloper mentioned this pull request Oct 6, 2020

Traduzir "guide/instance.md" vuejs-br/docs-next#170

Closed


		## Methods

		You might be tempted to put functions in your `data` so that you can use them as methods:


		## Data Properties

		The `data` function for a component is called as part of creating a new component instance. It should return an object, which Vue wraps in its reactivity system and stores on the instance as `$data`. For convenience, any top-level properties of that object are also exposed directly via the component instance:

	The `data` function for a component is called as part of creating a new component instance. It should return an object, which Vue wraps in its reactivity system and stores on the instance as `$data`. For convenience, any top-level properties of that object are also exposed directly via the component instance:
	The `data` property for a component is called as part of creating a new component instance. It is a function that should return an object, which Vue will then wrap in its reactivity system and store on the component instance as `$data`. For convenience, any top-level properties of that object are also exposed directly via the component instance:

Rewrite of instance.md, introducing data-methods.md #514

Rewrite of instance.md, introducing data-methods.md #514

Uh oh!

Conversation

skirtles-code commented Sep 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Introduction

The Problem

My Attempted Solution

Uh oh!

NataliaTepluhina commented Sep 21, 2020

Uh oh!

NataliaTepluhina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skirtles-code left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skirtles-code commented Sep 23, 2020

Uh oh!

NataliaTepluhina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bencodezen left a comment

Choose a reason for hiding this comment

Uh oh!

skirtles-code commented Sep 28, 2020

Uh oh!

NataliaTepluhina left a comment

Choose a reason for hiding this comment

Uh oh!

NataliaTepluhina commented Oct 4, 2020

Uh oh!

Uh oh!

skirtles-code commented Sep 20, 2020 •

edited

Loading